Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove additional zero byte when writing pub der #1258

Merged
merged 1 commit into from
Jun 29, 2018
Merged

remove additional zero byte when writing pub der #1258

merged 1 commit into from
Jun 29, 2018

Conversation

RonEld
Copy link
Contributor

@RonEld RonEld commented Jan 7, 2018

Description

The key_app writer writes wrong public key, in der format.
The reason is a leading zero byte, which is not a valid ASN1 tag
Fixes #1257

Status

READY

Requires Backporting

NO
This is only in a sample application

Steps to test or reproduce

  • Generate a key pair
  • use key_app_writer to write a public key in der format
  • try parsing the public key
./gen_key type=rsa rsa_keysize=4096 filename=rsa_4096_private.der format=der
./key_app_writer mode=private filename=rsa_4096_private.der output_mode=public output_format=der output_file=rsa_4096_public.der
./key_app mode=public filename=rsa_4096_public.der

remove additional zero byte when writing pub der
bb51cb3 
Remove `- 1` for setting location of output buffer,
which added a leading zero which cause failure in ASN1 parsing.
Fixes Mbed-TLS#1257
@RonEld RonEld added bug mbed TLS team needs-review Every commit must be reviewed by at least two team members, labels Jan 7, 2018
AndrzejKurek
AndrzejKurek approved these changes Jan 8, 2018
View reviewed changes
Copy link
Contributor

@AndrzejKurek AndrzejKurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it, the key_app does not print an error after the changes.

@simonbutcher simonbutcher added the needs-backports Backports are missing or are pending review and approval. label May 29, 2018
@simonbutcher
Copy link
Contributor

This is a minor bug but should still be fixed in the maintenance branches assuming it's broken there too. Fortunately the fix is easy to backport too.

@simonbutcher
Copy link
Contributor

retest

This was referenced Jun 26, 2018
Merged
Merged
@RonEld
Copy link
Contributor Author

RonEld commented Jun 26, 2018

Backports available in #1794 and #1795

mazimkhan
mazimkhan approved these changes Jun 26, 2018
View reviewed changes
Copy link

@mazimkhan mazimkhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@simonbutcher simonbutcher added approved Design and code approved - may be waiting for CI or backports component-x509 and removed needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members, labels Jun 27, 2018
@simonbutcher
Copy link
Contributor

Needs a ChangeLog entry. Can be fixed in gatekeeping.

@RonEld
Copy link
Contributor Author

RonEld commented Jun 28, 2018

@sbutcher-arm Do we add ChangeLog entries for changes in the example applications?

@simonbutcher simonbutcher merged commit bb51cb3 into Mbed-TLS:development Jun 29, 2018
@cwalther cwalther mentioned this pull request Nov 28, 2018
Merged
2 tasks
mpg pushed a commit to mpg/mbedtls that referenced this pull request Aug 29, 2024
@gilles-peskine-arm
Merge pull request Mbed-TLS#1258 from gilles-peskine-arm/psa-drbg-no-…
ea178dd 
…hmac-document-2.28

Backport 2.28: Document that MBEDTLS_PSA_HMAC_DRBG_MD_TYPE does not force HMAC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndrzejKurek AndrzejKurek AndrzejKurek approved these changes

@mazimkhan mazimkhan mazimkhan approved these changes

Assignees
No one assigned
Labels
approved Design and code approved - may be waiting for CI or backports bug component-x509
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RonEld @simonbutcher @mazimkhan @AndrzejKurek